Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editor: Create own sub-registry in default EditorProvider use #15989

Merged
merged 1 commit into from
Jun 10, 2019

Conversation

aduth
Copy link
Member

@aduth aduth commented Jun 4, 2019

Extracted from #14715
Related: #14678, #14367

This pull request seeks to introduce automated sub-registry creation for the EditorProvider component. It is identical in purpose and in implementation to the BlockEditorProvider enhancements merged in #14678.

This will bring consistency in editor creation, where #14715 proposes to reimplement reusable blocks using its own rendering of EditorProvider to leverage both the blocks-editing behaviors of @wordpress/block-editor, and the post lifecycle management of @wordpress/editor.

Implementation Notes:

This introduces API changes to @wordpress/editor and @wordpress/block-editor in the form of exposing their respective base data store configurations. For additional context, this was explored and discussed previously in #14367 (comment) .

Testing Instructions:

There should be no regressions in standard use of the editor.

You may place a breakpoint at any code within @wordpress/editor which has access to the registry to confirm there is no effective difference in reference between master and this branch, since sub-registries should only be created when not at the top-level.

registry.select === wp.data.select

import { storeConfig } from '../../store';
import applyMiddlewares from '../../store/middlewares';

const withRegistryProvider = createHigherOrderComponent(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like I already wrote this code before. Do you think there's value in exposing a generic version of this in the data module? (I'm on the fence personally)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like I already wrote this code before. Do you think there's value in exposing a generic version of this in the data module? (I'm on the fence personally)

It is identical to the one in block-editor, yes. I'm not sure either about making it generic. I'm not a huge fan of the fact we have to do a null return render, and I don't know that if that was possible to avoid, we'd be able to change it while maintaining the implementation as being a higher-order component. I was inclined to leave it for future consideration.

@@ -91,6 +91,7 @@ class Editor extends Component {
settings={ editorSettings }
post={ post }
initialEdits={ initialEdits }
useSubRegistry={ false }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this prop should be passed down from editor/provider to block-editor/provider as well right (unless we access the block-editor store from the editor components (which we probably do on a second thought but maybe we shouldn't) ?

Copy link
Member Author

@aduth aduth Jun 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this prop should be passed down from editor/provider to block-editor/provider as well right (unless we access the block-editor store from the editor components (which we probably do on a second thought but maybe we shouldn't) ?

Right now we just explicitly pass false:

Which I think is how we want it to be:

  • If you wanted your own block editor with its own subregistry, you'd render BlockEditorProvider yourself
  • If you wanted your own post editor with its own subregistry, you'd render EditorProvider which would create a subregistry. The EditorProvider would pass useSubRegistry={ false } to its BlockEditorProvider, but that block editor still operates in the same subregistry as created for the EditorProvider

In the end, we get a flatter hierarchy of registries. For the main post editor, we still only have a single registry.

For what becomes of the embedded editor in #14715, there is a subregistry which is used both by the EditorProvider and its BlockEditorProvider.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks for the explanation.

@aduth aduth merged commit a3ef960 into master Jun 10, 2019
@aduth aduth deleted the update/editor-provider-sub-registry branch June 10, 2019 15:53
@gziolo gziolo added this to the Gutenberg 6.0 milestone Jun 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Editor /packages/editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants